-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Defer tail call ret ty equality to check_tail_calls #144915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Defer tail call ret ty equality to check_tail_calls #144915
Conversation
|
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
r? lcnr r=me after nits |
That doesn't seem to track for me 🤔 The only reason we care about signature equality is that we need ABIs to match exactly, but ABIs are completely unaffected by lifetimes, so for the purpose of lifetimes we can do the same as normal call+ret. |
They are affected by higher-ranked lifetimes, though, and higher-ranked subtyping is different than higher-ranked equality. So what I'm saying is that checking this the way that normal return types are checked is (theoretically) insufficient since they don't enforce return type equality, but just a subtyping relationship between the thing being returned by the callee and the thing being returned by the caller. That being said, do we even have a case where a non-invariant arg causes a struct or something to have a different layout so that a subtyping operation would change its layout? |
A type and its subtype always has the same layout and ABI, since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like a reasonable step. r=us with nits
☔ The latest upstream changes (presumably #145020) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@rustbot author |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
e42f3ae
to
6a088fd
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
@bors r+ |
…quality, r=WaffleLapkin,lcnr Defer tail call ret ty equality to check_tail_calls Fixes rust-lang#144892. Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons. Firstly, we were using subtyping instead of equality in the HIR typeck code: https://github.com/rust-lang/rust/blob/e1b9081e699065badfc1a9419ec9566e5c8615c4/compiler/rustc_hir_typeck/src/expr.rs#L1096 We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions. That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying. All of this seems like a lot of work, and we already are *manually* checking argument equality. Let's just extend the `check_tail_calls` to account for mismatches in return types anyways. r? `@WaffleLapkin`
Rollup of 5 pull requests Successful merges: - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145415 (std_detect: RISC-V: implement implication to "C") - #145647 (miri subtree update) - #145650 (Fix JS search scripts path) r? `@ghost` `@rustbot` modify labels: rollup
…quality, r=WaffleLapkin,lcnr Defer tail call ret ty equality to check_tail_calls Fixes rust-lang#144892. Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons. Firstly, we were using subtyping instead of equality in the HIR typeck code: https://github.com/rust-lang/rust/blob/e1b9081e699065badfc1a9419ec9566e5c8615c4/compiler/rustc_hir_typeck/src/expr.rs#L1096 We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions. That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying. All of this seems like a lot of work, and we already are *manually* checking argument equality. Let's just extend the `check_tail_calls` to account for mismatches in return types anyways. r? ``@WaffleLapkin``
Rollup of 12 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144443 (Make target pointer width in target json an integer) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145415 (std_detect: RISC-V: implement implication to "C") - #145573 (Add an experimental unsafe(force_target_feature) attribute.) - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145674 (Enable triagebot `[review-changes-since]` feature) Failed merges: - #145647 (miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup
…quality, r=WaffleLapkin,lcnr Defer tail call ret ty equality to check_tail_calls Fixes rust-lang#144892. Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons. Firstly, we were using subtyping instead of equality in the HIR typeck code: https://github.com/rust-lang/rust/blob/e1b9081e699065badfc1a9419ec9566e5c8615c4/compiler/rustc_hir_typeck/src/expr.rs#L1096 We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions. That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying. All of this seems like a lot of work, and we already are *manually* checking argument equality. Let's just extend the `check_tail_calls` to account for mismatches in return types anyways. r? ```@WaffleLapkin```
Rollup of 14 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144443 (Make target pointer width in target json an integer) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145137 (Consolidate panicking functions in `slice/index.rs`) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145297 (fix(debuginfo): handle false positives in overflow check) - #145415 (std_detect: RISC-V: implement implication to "C") - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145674 (Enable triagebot `[review-changes-since]` feature) - #145678 (Fix typo in docstring) r? `@ghost` `@rustbot` modify labels: rollup
…quality, r=WaffleLapkin,lcnr Defer tail call ret ty equality to check_tail_calls Fixes rust-lang#144892. Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons. Firstly, we were using subtyping instead of equality in the HIR typeck code: https://github.com/rust-lang/rust/blob/e1b9081e699065badfc1a9419ec9566e5c8615c4/compiler/rustc_hir_typeck/src/expr.rs#L1096 We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions. That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying. All of this seems like a lot of work, and we already are *manually* checking argument equality. Let's just extend the `check_tail_calls` to account for mismatches in return types anyways. r? ````@WaffleLapkin````
Rollup of 15 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145137 (Consolidate panicking functions in `slice/index.rs`) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145297 (fix(debuginfo): handle false positives in overflow check) - #145415 (std_detect: RISC-V: implement implication to "C") - #145590 (Prevent impossible combinations in `ast::ModKind`.) - #145621 (Fix some doc typos) - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145674 (Enable triagebot `[review-changes-since]` feature) - #145678 (Fix typo in docstring) r? `@ghost` `@rustbot` modify labels: rollup
…quality, r=WaffleLapkin,lcnr Defer tail call ret ty equality to check_tail_calls Fixes rust-lang#144892. Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons. Firstly, we were using subtyping instead of equality in the HIR typeck code: https://github.com/rust-lang/rust/blob/e1b9081e699065badfc1a9419ec9566e5c8615c4/compiler/rustc_hir_typeck/src/expr.rs#L1096 We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions. That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying. All of this seems like a lot of work, and we already are *manually* checking argument equality. Let's just extend the `check_tail_calls` to account for mismatches in return types anyways. r? `````@WaffleLapkin`````
Rollup of 19 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145297 (fix(debuginfo): handle false positives in overflow check) - #145390 (Shorten some dependency chains in the compiler) - #145415 (std_detect: RISC-V: implement implication to "C") - #145525 (stdlib: Replace typedef -> type alias in doc comment) - #145590 (Prevent impossible combinations in `ast::ModKind`.) - #145593 (UnsafePinned::raw_get: sync signature with get) - #145621 (Fix some doc typos) - #145627 (Unconditionally-const supertraits are considered not dyn compatible) - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145673 (Add flock support for cygwin) - #145674 (Enable triagebot `[review-changes-since]` feature) - #145678 (Fix typo in docstring) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144915 - compiler-errors:tail-call-ret-ty-equality, r=WaffleLapkin,lcnr Defer tail call ret ty equality to check_tail_calls Fixes #144892. Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons. Firstly, we were using subtyping instead of equality in the HIR typeck code: https://github.com/rust-lang/rust/blob/e1b9081e699065badfc1a9419ec9566e5c8615c4/compiler/rustc_hir_typeck/src/expr.rs#L1096 We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions. That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying. All of this seems like a lot of work, and we already are *manually* checking argument equality. Let's just extend the `check_tail_calls` to account for mismatches in return types anyways. r? ``````@WaffleLapkin``````
Fixes #144892.
Currently the tail call signature check assumes that return types have been accounted for. However, this is not complete for several reasons.
Firstly, we were using subtyping instead of equality in the HIR typeck code:
rust/compiler/rustc_hir_typeck/src/expr.rs
Line 1096 in e1b9081
We could fix this, but it doesn't really do much for us anyways since HIR typeck doesn't care about regions.
That means, secondly, we'd need to fix the terminator type check in MIR typeck to account for variances, since tail call terminators need to relate their arguments invariantly to account for the "signature must be equal" rule. This seems annoying.
All of this seems like a lot of work, and we already are manually checking argument equality. Let's just extend the
check_tail_calls
to account for mismatches in return types anyways.r? @WaffleLapkin